Skip to content

Fix sticky scroll height calculation for variable line heights#3971

Merged
Christopher-Hermann merged 2 commits into
eclipse-platform:masterfrom
Christopher-Hermann:sticky
Jun 1, 2026
Merged

Fix sticky scroll height calculation for variable line heights#3971
Christopher-Hermann merged 2 commits into
eclipse-platform:masterfrom
Christopher-Hermann:sticky

Conversation

@Christopher-Hermann
Copy link
Copy Markdown
Contributor

@Christopher-Hermann Christopher-Hermann commented May 6, 2026

Description:
StickyScrollingControl was computing the sticky area height by multiplying a single line height by the number of sticky lines. This assumption breaks when content like emoji causes individual lines to render taller than the default — the canvas ends up too short, clipping the sticky lines visually.

calculateCanvasBounds now sums the actual height of each sticky line using getLineHeight(offset) instead of using a uniform getLineHeight().

Reproduce:
Create an XML file with the content below. Enable sticky scrolling and scroll a few lines:

<?xml version="1.0" encoding="UTF-8"?>
<test>
	<test2 name="👍">
		<test3 name="👎" />
		<!--
    Extended comment section 
    to demonstrate 
    sticky scrolling 
    functionality
  -->
	</test2>
</test>
image

Follow up of #3748

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026 •
edited
Loading

Test Results

   864 files  ±0     864 suites  ±0   51m 7s ⏱️ - 1m 23s
 7 989 tests +1   7 746 ✅ +1  243 💤 ±0  0 ❌ ±0 
20 421 runs  +3  19 766 ✅ +3  655 💤 ±0  0 ❌ ±0 

Results for commit 79ed6d3. ± Comparison against base commit 1e6a7c9.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Copy Markdown
Member

iloveeclipse commented May 6, 2026

There is a test failure in testCanvasBoundsHeightAdjustsForVariableLineHeights (org.eclipse.ui.internal.texteditor.stickyscroll.StickyScrollingControlTest)

@Christopher-Hermann Christopher-Hermann force-pushed the sticky branch 2 times, most recently from f731e13 to 2e83544 Compare May 6, 2026 11:31
@Christopher-Hermann
Copy link
Copy Markdown
Contributor Author

Christopher-Hermann commented May 6, 2026

There is a test failure in testCanvasBoundsHeightAdjustsForVariableLineHeights (org.eclipse.ui.internal.texteditor.stickyscroll.StickyScrollingControlTest)

Fixed in the latest version 👍

@iloveeclipse iloveeclipse requested a review from Copilot May 6, 2026 15:30
@BeckerWdf
Copy link
Copy Markdown
Member

BeckerWdf commented May 18, 2026

@HeikoKlare: We missed to merge this last week for M3 - thanks to the bridge day.
The change looks straightforward.
What do you think? Is this something we still can merge for RC1?

@Christopher-Hermann
Fix sticky scroll height calculation for variable line heights
5686df7 
When emoji or other content affects line heights, sticky scroll was calculating the total height wrong by assuming all lines were the same. Now it gets the actual height for each line instead. Also fixes the height check for limiting visible lines.
@eclipse-platform-bot
Copy link
Copy Markdown
Contributor

eclipse-platform-bot commented Jun 1, 2026

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.ui.editors/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch 
From 6ff9273d8cda914b453f40604c3328ae6b86d9a2 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Mon, 1 Jun 2026 06:24:36 +0000
Subject: [PATCH] Version bump(s) for 4.41 stream


diff --git a/bundles/org.eclipse.ui.editors/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.editors/META-INF/MANIFEST.MF
index 8e375d7187..8cdb3e3954 100644
--- a/bundles/org.eclipse.ui.editors/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.editors/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.ui.editors; singleton:=true
-Bundle-Version: 3.22.0.qualifier
+Bundle-Version: 3.22.100.qualifier
 Bundle-Activator: org.eclipse.ui.internal.editors.text.EditorsPlugin
 Bundle-ActivationPolicy: lazy
 Bundle-Vendor: %providerName
-- 
2.54.0

Further information are available in Common Build Issues - Missing version increments.

@Christopher-Hermann Christopher-Hermann merged commit 8ad0c90 into eclipse-platform:master Jun 1, 2026
18 checks passed
@Christopher-Hermann Christopher-Hermann deleted the sticky branch June 1, 2026 07:22
@BeckerWdf BeckerWdf added this to the 4.41 M1 milestone Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BeckerWdf BeckerWdf BeckerWdf approved these changes

Copilot code review Copilot Awaiting requested review from Copilot

Assignees

@Christopher-Hermann Christopher-Hermann

Labels

None yet

Projects

None yet

Milestone

4.41 M1

Development

Successfully merging this pull request may close these issues.

4 participants

@Christopher-Hermann @iloveeclipse @BeckerWdf @eclipse-platform-bot